-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
workflows/stale: fix workflow syntax #65692
Conversation
Might as well. It's not like with |
Instead I'd suggest making it run on |
A few questions (due to me being very new to GitHub actions):
Sorry, not familiar enough with
Okay, I think I understand what you mean. You're suggesting that I change to this? on:
push:
schedule:
# Once every day
- cron: "0 0 * * *" You did say:
I'm not sure how to restrict to events that change the workflow file. Like this, maybe? on:
push:
paths:
- .github/workflows/stale.yml
schedule:
# Once every day
- cron: "0 0 * * *"
Sorry, I'm not sure what you mean here. |
That looks like what we want.
It probably doesn't run from a fork, so if you want to test you can |
👍 Thanks!
Oh, I see. You're saying that it's not running because this PR is from a fork, right? If I had instead opened this PR from another branch in the main repo it would have run? So pushing to the |
Yes
It would with that config
Yeah, that sounds about right |
Thanks! I'll give that a try |
It looks like that worked for issues. Here are a few issues that were marked stale: #59802, #62084. Here's an issue that was closed: #64167. It also looks like it stayed away from It didn't run on any PRs. Looking at the logs, it found stale PRs but didn't mark them saying Looking good! |
Okay, looks like this works for PRs now! Here are a few PRs that were marked as stale: #63160, #63161. Here are some PRs that were closed: #64698, #64699. While this is great, I was expecting it to close/mark a few more PRs. For example, #61838 doesn't seem like it's had any activity for over a month but it wasn't marked. Maybe there's a limit to how many can be marked in one go? Edit: this shows up in the logs:
From the action docs, it looks like the Edit 2: Yep, just manually ran again and more were marked (and we reached the limit again). I'll bump the limit for these initial runs but won't modify it in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, nice work @Rylan12. If it'd be possible could you add this to Homebrew/brew after this is merged here?
(similarly if any of these others seem not to be working it'd be good to investigate and replace with actions?) |
One final thing to keep in mind: This action is supposed to remove the stale label if there's a comment from a user who is not the "actor" (does that mean author? not sure...). However, if this is run only once a day the label won't be automatically removed until the next run (which is the next day). I think that's fine, right? |
Absolutely. Just wanted to try it in one repo at a time.
I've glanced at the others. We can probably adapt the stale action to the no-response workflow. The others will probably need a different action. I can do some research to see if anything looks right. Should/is there a way to have these configurations live in Homebrew/.github instead of in each individual repo? Not sure if it saves us any work. They won't automatically run anyway. |
Yup, seems to be!
Perfect ❤️
I don't think so, unfortunately 😭 |
Alright, sounds good. I'll merge this. I'm currently in the middle of migrating the audit exceptions so once I'm done with that (or at least at a stopping point) I'll add the stale workflow to brew too (and look into the others). No reason to keep those old config files in |
Agreed! |
I believe this means that the |
Possibly. That would certainly make sense. I checked the source code and it looks like it excludes comments by In #63176, though, a user commented after the
This is surprising to me because:
Edit: I found another example where the stale label is removed (as expected): #62722 (comment) Edit 2: I looked into it a little more. Based on actions/stale#73 (comment), actions/stale#192, and actions/stale#21 (comment), it seems that the "actor" in this case is the last person to modify the workflow file i.e. me. That doesn't explain it but is... interesting |
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingbrew install <formula>
)?The stale action that I added yesterday failed with a message saying:
You can see the failed workflow run here
This PR (hopefully) fixes the syntax to match the examples given in the upstream repo (not sure why I didn't try to match them for the first round...)
For testing purposes, is it worth adding a
workflow_dispatch
trigger to allow manual testing? I don't think that is something we need to be able to do once the action seems to be working, but may be helpful if we continue to have problems. I'll hold off for now, but figured I'd through the idea out there.CC: @jonchang